Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Correcting coverage calculations #310

Merged
merged 35 commits into from
Aug 24, 2023
Merged

Correcting coverage calculations #310

merged 35 commits into from
Aug 24, 2023

Conversation

anilbey
Copy link
Contributor

@anilbey anilbey commented Aug 24, 2023

Description

As reported in #306, the python coverage reposting of efel was wrong. It was only detecting the test modules' coverage.

This PR corrects the coverage calculations for Python and also starts measuring coverage for C++. Efel is 58.7% C++ and
39.4% Python.

The whole list of changes include:

  • Report C++ coverage
  • Report Python coverage
  • Integrate them to tox
  • Integrate them via codecov
  • Significantly faster tox test environment
  • Move test module out of the deployed package to not to deploy tests and test dependencies
  • Remove outdated repository files

Current status of coverage

We have ~%96 Python coverage and ~%41 C++ branch coverage.

Python

Name                                 Stmts   Miss  Cover   Missing
 ------------------------------------------------------------------
 efel/__init__.py                         5      0   100%
 efel/api.py                            163      2    99%   267, 344
 efel/io.py                             108      7    94%   40, 63, 74, 180-181, 211-212
 efel/pyfeatures/__init__.py              2      0   100%
 efel/pyfeatures/pyfeatures.py          132     12    91%   176, 206, 215, 220, 236-247, 269, 282
 efel/settings.py                        13      0   100%
 efel/units/__init__.py                   6      0   100%
 tests/plot_expectedresults_diff.py      34     34     0%   3-87
 tests/test_allfeatures.py              101      4    96%   181, 185, 191-193
 tests/test_basic.py                   1827      8    99%   122, 2718, 3211, 3229, 3238, 3314, 3463, 3697
 tests/test_cppcore.py                   94      0   100%
 tests/test_io.py                       161      5    97%   44, 52, 59-60, 63
 tests/test_pyfeatures.py                95      0   100%
 tests/test_units.py                      8      0   100%
 tests/update_expectedresults.py         19     19     0%   3-68
 tests/update_featurenames.py            13     13     0%   3-52
 ------------------------------------------------------------------
 TOTAL                                 2781    104    96%

C++

branches: 41.4% (6394 out of 15450)

@anilbey
remove unnecessary files
c0abd19
@anilbey
Merge pull request #2 from anilbey/test-tox
d707ee5 
remove unnecessary files
@anilbey
remove builddist
accabbb
@anilbey
remove py37 from test.yml
b024c55
@anilbey
simplify setup.py
438dfe4
@anilbey
Merge pull request #3 from anilbey/tox-remove
5ac9fb2 
remove builddist
@anilbey
remove unused passenv in tox
2bd6c75
@anilbey
move tests out of package
356df25
@anilbey
update Makefile
88b16d6
@anilbey
update Makefile remove old nosetest commands
740b55a
@anilbey
delete .codeclimate.yml
15d05aa
@anilbey
stay with the old versioneer
464d61e
@anilbey
update .gitignore
0f0a887
@anilbey
update make clean paths after moving tests out
a35e28c
@anilbey
Correctly calculating the coverage
1b1078d
@anilbey
add back bdist_wheel to setup.cfg
a4cd401
@anilbey
Revert "add back bdist_wheel to setup.cfg"
50ebfea 
This reverts commit 37da209.
@anilbey
add back docs to test.yml
673168c
@anilbey
tox fix envdir
c638a67
@anilbey
tox pytest add -sx flags
9ff3a3f
@anilbey
use coverage instead of pytest-cov
3cc716c
@anilbey
revert envdir
2fa807c
@anilbey
revert deps
4fd463a
@anilbey
revert pytest
795b224
@anilbey
add usedevelop=True
2b366a3
@anilbey
use pytest-cov back again
6ad686d
@anilbey
merge tox into master
0f98840
@anilbey
upload coverage only once
e7eaf39
@anilbey
remove py37 from gh actions
e5c5188
@anilbey
report cpp coverage as well as python
b29a3bb
@anilbey
performance: separate coverage tox environment
16b2b58
@anilbey
performance pytest-xdist for test tox env
7f74680
@anilbey
separate coverage ci action
eb39a2a 
removed unnecessary if check in coverage yaml

switch  to codecov-action@v3

add .codecov.yml with branch coverage gcov

add --branches flag to gcovr

branch detection: no codecov.yml

gcovr treat pratials as hits

gcov to give txt output

delete .codecov.yml

mention txt format gcovr

try json format for gcovr

gcovr output -> xml
@anilbey
rename Python coverage-> coverage since we add c++ as well
52f4e61
@anilbey anilbey mentioned this pull request Aug 24, 2023
Closed
AurelienJaquier
AurelienJaquier approved these changes Aug 24, 2023
View reviewed changes
Copy link
Collaborator

@AurelienJaquier AurelienJaquier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent!

.github/workflows/test.yml Outdated Show resolved Hide resolved
@anilbey
Copy link
Contributor Author

anilbey commented Aug 24, 2023

The old "test (3.7) Expected — Waiting for status to be reported" is blocking us from merging.

@anilbey
Copy link
Contributor Author

anilbey commented Aug 24, 2023

@wvangeit the old test check is blocking it seems to be due to the branch protection rules https://github.com/orgs/community/discussions/26698#discussioncomment-3252954. Could you update them please?

If you'll merge could you also squash the commits this time? I created lots of revert commits while testing, they will look confusing in the git log I think.

@AurelienJaquier
Copy link
Collaborator

ok finally found how to remove the check. @anilbey it's good to be merged now ;)

@anilbey anilbey merged commit 3537cf3 into BlueBrain:master Aug 24, 2023
6 checks passed
@anilbey
Copy link
Contributor Author

anilbey commented Aug 24, 2023

Great, thanks @AurelienJaquier

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AurelienJaquier AurelienJaquier AurelienJaquier approved these changes

@wvangeit wvangeit wvangeit approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@anilbey @AurelienJaquier @wvangeit